Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #563 More flexible way of node renaming #817

Merged
merged 13 commits into from
Jun 26, 2020
Merged

Conversation

stepanzharychevbroadcom
Copy link
Contributor

Proposed changes

This branch adds more flexible way of node renaming. It includes verification file status and allows to save it before rename will happen, so your changes won't be lost.

Milestone: 1.7

Types of changes

What types of changes does your code introduce to Zowe Explorer?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Updates to Documentation or Tests (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This checklist will be used as reference for both the contributor and the reviewer

  • I have read the CONTRIBUTOR GUIDANCE wiki
  • PR title follows Conventional Commits Guidelines
  • PR Description is included
  • npm run vscode:prepublish has been executed
  • All checks have passed (DCO, Jenkins and Code Coverage)
  • I have added unit test and it is passing
  • I have added integration test and it is passing
  • There is coverage for the code that I have added
  • I have tested it manually and there are no regressions found
  • I have added necessary documentation (if appropriate)
  • Any PR dependencies have been merged and published (if appropriate)

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

Signed-off-by: Stepan Zharychev <stepan.zharychev@broadcom.com>
…fixes for closing/saving files on renaming.

Signed-off-by: Stepan Zharychev <stepan.zharychev@broadcom.com>
@zdmullen
Copy link
Contributor

Also covers issue #567, it appears.

Copy link
Contributor

@katelynienaber katelynienaber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works for me in all but a few instances:

  • If I rename a dataset node in favorites, the node is renamed successfully. But if the node is visible in the regular list (not in favorites) the new name doesn't appear unless I refresh, or hide/reveal the node again.
  • I can't rename a USS node in favorites
  • If I rename a USS node which is favorited, the favorited version of the node isn't renamed. If I click on the renamed node's favorited version, it gives an error. The node is thereafter renamed in favorites, but even so I can't open it.

Signed-off-by: Stepan Zharychev <stepan.zharychev@broadcom.com>
@@ -14,12 +14,18 @@ import { createUSSTree, USSTree } from "../../../src/uss/USSTree";
import { ZoweUSSNode } from "../../../src/uss/ZoweUSSNode";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like all changes in this file are related to formatting. Perhaps it is time, that we implement prettier #815

@phaumer phaumer changed the title Issue #563 Issue #563 More flexible way of node renaming Jun 10, 2020
@VitGottwald
Copy link
Contributor

@stepanzharychevbroadcom could you look at the issues @katelynienaber found out above?

stepanzharychevbroadcom and others added 5 commits June 12, 2020 12:21
Signed-off-by: Stepan Zharychev <stepan.zharychev@broadcom.com>
Signed-off-by: Stepan Zharychev <stepan.zharychev@broadcom.com>
Signed-off-by: Stepan Zharychev <stepan.zharychev@broadcom.com>
Signed-off-by: Stepan Zharychev <stepan.zharychev@broadcom.com>
@stepanzharychevbroadcom
Copy link
Contributor Author

@zdmullen @katelynienaber I fixed first issue, but 2 others will need the separate ticket. Problem there is deeper than this branch, logic of USS favorite adding ignores current search pattern. So if you operate under /tmp/test-folder pattern and add test.txt file to favourites it's full path will be saved as /test.txt which is incorrect and causes error, so it should be reworked.

@katelynienaber
Copy link
Contributor

@stepanzharychevbroadcom Thank you ^^ The first issue looks fine now. Could you link to the issue created for other 2 items? I don't want to lose them :) I will approve after this.

…-zowe into feature/issue-563-new

Signed-off-by: zFernand0 <fernando.rijocedeno@broadcom.com>

# Conflicts:
#	src/uss/ZoweUSSNode.ts
zFernand0
zFernand0 previously approved these changes Jun 26, 2020
Copy link
Member

@zFernand0 zFernand0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@katelynienaber
Copy link
Contributor

katelynienaber commented Jun 26, 2020

Posting link to opened issue for bugs 2 & 3 I found above: #911

Signed-off-by: zFernand0 <fernando.rijocedeno@broadcom.com>
katelynienaber
katelynienaber previously approved these changes Jun 26, 2020
Copy link
Contributor

@katelynienaber katelynienaber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue 1 seems to be fixed and @zFernand0 has opened an issue for bugs 2 & 3, thanks guys! LGTM :D

Signed-off-by: zFernand0 <fernando.rijocedeno@broadcom.com>
@codecov
Copy link

codecov bot commented Jun 26, 2020

Codecov Report

Merging #817 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #817   +/-   ##
=======================================
  Coverage   91.06%   91.06%           
=======================================
  Files          51       51           
  Lines        5283     5283           
  Branches     1056     1056           
=======================================
  Hits         4811     4811           
  Misses        468      468           
  Partials        4        4           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c59bf21...e12823b. Read the comment docs.

@katelynienaber katelynienaber self-requested a review June 26, 2020 08:49
Copy link
Contributor

@katelynienaber katelynienaber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving 1 more time :)

@zFernand0
Copy link
Member

Codecov Report

Merging #817 into master will decrease coverage by 1.26%.
The diff coverage is 66.96%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #817      +/-   ##
==========================================
- Coverage   90.78%   89.51%   -1.27%     
==========================================
  Files          49       51       +2     
  Lines        4364     4494     +130     
  Branches      767      791      +24     
==========================================
+ Hits         3962     4023      +61     
- Misses        398      467      +69     
  Partials        4        4              

Impacted Files Coverage Δ
src/config/constants.ts 100.00% <ø> (ø)
src/uss/actions.ts 76.47% <29.41%> (-1.24%) ⬇️
src/dataset/actions.ts 86.04% <30.00%> (-8.70%) ⬇️
src/dataset/DatasetTree.ts 81.77% <66.66%> (-0.33%) ⬇️
src/extension.ts 85.77% <68.75%> (-1.26%) ⬇️
src/utils/workspace.ts 79.62% <79.16%> (ø)
src/uss/ZoweUSSNode.ts 84.78% <90.90%> (-0.04%) ⬇️
src/uss/USSTree.ts 88.23% <100.00%> (+0.08%) ⬆️
... and 1 more
Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c59bf21...e12823b. Read the comment docs.

Is this for real?
When you click the Continue to review full report at Codecov., it just says 91.07%

@zFernand0
Copy link
Member

Merging #817 into master will decrease coverage by 1.26%.
The diff coverage is 66.96%.

Is this for real?
When you click the Continue to review full report at Codecov., it just says 91.07%

Nvm, it's all good now!

@zFernand0 zFernand0 merged commit d054444 into master Jun 26, 2020
@zFernand0 zFernand0 deleted the feature/issue-563-new branch June 26, 2020 09:39
@zFernand0
Copy link
Member

@stepanzharychevbroadcom next time, please use the complete the template 😋

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

USS Rename - File Opening flow
5 participants